Skip to content

BLE upgrades #7867

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 18 commits into from
Closed

Conversation

PilnyTomas
Copy link
Contributor

@PilnyTomas PilnyTomas commented Feb 20, 2023

General:

BREAKING CHANGE! Changed API return and parameter type from std::tring to Arduino style String.
Renamed library name from ESP32 BLE Arduino to simple BLE.
Created a new function getFrame which returns complete data for BLEAdvertisementData::addData().
Renamed example files - removed prefix BLE_ - BLE5_ is left unchanged.
Beacon_Scanner example updated to reflect changes in API - shortened by 28 lines on TLM and URL part.
Changed UUID data type from uint16_t to BLEUUID class

Eddysstone URL:

Created a new constructor which takes as an argument a BLEAdvertisedDevice object and initializes URL data from the payload.
Created a new function BLEEddystoneURL::setSmartURL which encodes the full URL structure.

Static frame data no longer needs to be written by the user - they are initialized by the constructor.
Function BLEEddystoneURL::setPower now accepts esp_power_level_t.
Updated EddystoneURL_Beacon example to reflect changes in API - shortened by 83 lines.
Added comments with explanations to functions setData and setURL.

Tests:
EddystoneURL_Beacon with the following URLs and read them in Beacon_Scanner example and using an Android app nRF Connect for Mobile.

char unprintable[] = {0x01, 0xFF, 0xDE, 0xAD};
String URL[] = {
"http://www.espressif.com/", // prefix 0x00, suffix 0x00
"https://www.texas.gov",     // prefix 0x01, suffix 0x0D
"http://en.mapy.cz",         // prefix 0x02, no valid suffix
"https://arduino.cc",        // prefix 0x03, no valid suffix
"google.com",                // URL without specified prefix - the function will assume default prefix "http://www." = 0x00
"diginfo.tv",                // URL without specified prefix - the function will assume default prefix "http://www." = 0x00
"http://www.URLsAbove17BytesAreNotAllowed.com", // Too long URL - setSmartURL() will return 0 = ERR
String(unprintable) // Unprintable characters - setSmartURL() will return 0 = ERR

Eddystone TLM:

Created a new constructor which takes as an argument a BLEAdvertisedDevice object and initializes TLM data from the payload.
Added comments with explanations to functions setData and setVolt

TODO: test all examples if they are properly working - changes related to the String might have broken something!

Related task: #7841

@PilnyTomas PilnyTomas added the Area: BLE Issues related to BLE label Feb 20, 2023
@PilnyTomas PilnyTomas added this to the 3.0.0 milestone Feb 20, 2023
@PilnyTomas PilnyTomas requested a review from SuGlider February 20, 2023 11:55
@PilnyTomas PilnyTomas self-assigned this Feb 20, 2023
@PilnyTomas PilnyTomas marked this pull request as draft February 20, 2023 12:13
@SuGlider
Copy link
Collaborator

@PilnyTomas - There are commits from 2022 in this PR. Is it right?

image

@PilnyTomas PilnyTomas changed the title Draft: BLE Eddystone URL fix Draft: BLE Eddystone URL upgrade Feb 23, 2023
@PilnyTomas PilnyTomas changed the title Draft: BLE Eddystone URL upgrade Draft: BLE Eddystone upgrade Feb 23, 2023
@PilnyTomas
Copy link
Contributor Author

PilnyTomas commented Feb 23, 2023

There are commits from 2022 in this PR. Is it right?

To be honest, I don't know how they got there, but considering that they are mostly just merges of master it shouldn't be a problem. The only change was adding the vector #include, but as is not needed I have removed it in one of the following commits.

@PilnyTomas PilnyTomas changed the title Draft: BLE Eddystone upgrade Draft: BLE upgrades Mar 20, 2023
@PilnyTomas PilnyTomas marked this pull request as ready for review March 21, 2023 12:16
@PilnyTomas PilnyTomas requested a review from me-no-dev March 22, 2023 10:46
@VojtechBartoska VojtechBartoska changed the title Draft: BLE upgrades BLE upgrades Mar 24, 2023
@CLAassistant
Copy link

CLAassistant commented May 6, 2023

CLA assistant check
All committers have signed the CLA.

@PilnyTomas PilnyTomas changed the base branch from master to esp-idf-v5.1-libs July 4, 2023 11:10
Copy link
Collaborator

@lucasssvaz lucasssvaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested all examples. Everything works as expected. Just some indentation problems, mainly where std::strings were replaced by Strings

Co-authored-by: Lucas Saavedra Vaz <[email protected]>
@VojtechBartoska
Copy link
Contributor

Related to the BLE refactoring: #7702

@me-no-dev me-no-dev changed the base branch from esp-idf-v5.1-libs to master October 5, 2023 12:26
@me-no-dev me-no-dev changed the base branch from master to esp-idf-v5.1-libs October 5, 2023 12:26
@me-no-dev me-no-dev mentioned this pull request Oct 5, 2023
@me-no-dev
Copy link
Member

Closed in favor of #8724

@me-no-dev me-no-dev closed this Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: BLE Issues related to BLE Status: Review needed Issue or PR is awaiting review
Projects
Development

Successfully merging this pull request may close these issues.

6 participants